feat: Add measured boot trust approvals#3464
Conversation
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds REST and Core support for creating, listing, and deleting measured-boot trusted machine and profile approvals, with validation, authorization, structured removal errors, routing, OpenAPI documentation, and automated tests. ChangesMeasurement trust approvals
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant APIClient
participant MeasurementTrustHandler
participant CoreGrpcProxy
participant MeasurementTrustRPC
participant MeasurementTrustDB
APIClient->>MeasurementTrustHandler: Create, list, or delete approval
MeasurementTrustHandler->>MeasurementTrustHandler: Validate request and authorize site
MeasurementTrustHandler->>CoreGrpcProxy: Execute measurement trust RPC
CoreGrpcProxy->>MeasurementTrustRPC: Process approval operation
MeasurementTrustRPC->>MeasurementTrustDB: Read or remove approval
MeasurementTrustDB-->>MeasurementTrustRPC: Return record or database error
MeasurementTrustRPC-->>CoreGrpcProxy: Return approval or structured error
CoreGrpcProxy-->>MeasurementTrustHandler: Return protobuf response
MeasurementTrustHandler-->>APIClient: Return HTTP response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3464.docs.buildwithfern.com/infra-controller |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-07-14 01:42:37 UTC | Commit: cc4c71c |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4c71c715
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
rest-api/api/pkg/api/model/measurementtrust.go (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModel approval-type as a named type with receiver methods instead of raw string + free helpers.
approvalTypeis currently a barestringvalidated via a hand-rolled switch (validateMeasurementTrustApprovalType) and converted via free functions (measurementTrustApprovalTypeToProto/FromProto). Per coding guidelines, this should be a named type with receiver methods, and validation should prefer ozzo's built-invalidation.Inrule (or afunc(interface{}) errorvalidator usable withvalidation.By) rather than a bespoke switch.♻️ Suggested refactor
-const ( - MeasurementTrustApprovalTypeOneshot = "Oneshot" - MeasurementTrustApprovalTypePersist = "Persist" -) +type MeasurementTrustApprovalType string + +const ( + MeasurementTrustApprovalTypeOneshot MeasurementTrustApprovalType = "Oneshot" + MeasurementTrustApprovalTypePersist MeasurementTrustApprovalType = "Persist" +) + +func (t MeasurementTrustApprovalType) ToProto() corev1.MeasurementApprovedTypePb { + if t == MeasurementTrustApprovalTypePersist { + return corev1.MeasurementApprovedTypePb_Persist + } + return corev1.MeasurementApprovedTypePb_Oneshot +}Then use
validation.Field(&r.ApprovalType, validation.Required, validation.In(MeasurementTrustApprovalTypeOneshot, MeasurementTrustApprovalTypePersist))insideValidateStruct.Also applies to the wildcard
MachineID != "*"check at Lines 81-85, which could becomevalidation.When(r.MachineID != "*", validationis.UUID.Error(...))insideValidateStructfor a single-pass validation.Also applies to: 72-99, 231-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rest-api/api/pkg/api/model/measurementtrust.go` around lines 17 - 20, Refactor approvalType in the measurement-trust model into a named type with receiver methods for proto conversion, replacing validateMeasurementTrustApprovalType and the free conversion helpers. Update ValidateStruct to validate ApprovalType with validation.Required and validation.In using the two approval constants, and express the MachineID wildcard exception with validation.When and the UUID rule in the same validation pass.Source: Coding guidelines
rest-api/openapi/spec.yaml (1)
1296-1329: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMissing pagination on the two new list endpoints.
Both
get-all-measurement-trusted-machineandget-all-measurement-trusted-profileonly acceptsiteIdand omitpageNumber/pageSize/orderByplus theX-Paginationresponse header, unlike essentially every otherget-all-*endpoint in this spec — including other Site-scoped resources such asget-all-sku.Persist-type approvals have no forced expiry, so this list can grow unbounded at a busy Site, and callers have no way to page through results.♻️ Suggested pagination addition (apply to both list operations)
parameters: - schema: type: string format: uuid name: siteId in: query required: true description: ID of the Site + - schema: + type: integer + example: 1 + default: 1 + minimum: 1 + in: query + name: pageNumber + description: Page number for pagination query + - schema: + type: integer + minimum: 1 + maximum: 100 + example: 20 + in: query + name: pageSize + description: Page size for pagination query responses: '200': description: Measured Boot trusted Machine approvals content: application/json: schema: type: array items: $ref: '`#/components/schemas/MeasurementTrustedMachine`' + headers: + X-Pagination: + schema: + type: string + example: '{"pageNumber":1,"pageSize":20,"total":30}' + description: Pagination result in JSON formatAlso applies to: 1423-1456
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rest-api/openapi/spec.yaml` around lines 1296 - 1329, Update both list operations, get-all-measurement-trusted-machine and get-all-measurement-trusted-profile, to accept the standard pageNumber, pageSize, and orderBy query parameters used by comparable get-all endpoints such as get-all-sku. Add the X-Pagination response header to each 200 response while preserving the existing siteId parameter and response schemas.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rest-api/api/pkg/api/model/measurementtrust.go`:
- Around line 126-180: Move the protobuf conversion logic from the free
functions APIMeasurementTrustedMachineFromProto and
APIMeasurementTrustedProfileFromProto onto FromProto receiver methods of their
respective API model structs. Replace the plural helpers
APIMeasurementTrustedMachinesFromProto and
APIMeasurementTrustedProfilesFromProto with named slice types and corresponding
FromProto receiver methods, preserving nil handling and element conversion
behavior.
- Around line 182-229: Replace MeasurementTrustedMachineRemoveProto and
MeasurementTrustedProfileRemoveProto with delete request DTOs containing
selector and path-derived ID fields, plus Validate and error-free ToProto
methods matching the existing create-request pattern. Populate these DTOs in the
handlers, run authorization before validation/conversion, then call Validate
followed by ToProto; preserve wildcard machine-ID handling and selector-specific
protobuf mapping.
---
Nitpick comments:
In `@rest-api/api/pkg/api/model/measurementtrust.go`:
- Around line 17-20: Refactor approvalType in the measurement-trust model into a
named type with receiver methods for proto conversion, replacing
validateMeasurementTrustApprovalType and the free conversion helpers. Update
ValidateStruct to validate ApprovalType with validation.Required and
validation.In using the two approval constants, and express the MachineID
wildcard exception with validation.When and the UUID rule in the same validation
pass.
In `@rest-api/openapi/spec.yaml`:
- Around line 1296-1329: Update both list operations,
get-all-measurement-trusted-machine and get-all-measurement-trusted-profile, to
accept the standard pageNumber, pageSize, and orderBy query parameters used by
comparable get-all endpoints such as get-all-sku. Add the X-Pagination response
header to each 200 response while preserving the existing siteId parameter and
response schemas.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c9957257-ea84-4d0f-b9bf-31302c09ffd4
⛔ Files ignored due to path filters (7)
rest-api/sdk/standard/api_measured_boot_trusted_machine.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/api_measured_boot_trusted_profile.gois excluded by!rest-api/sdk/standard/api_*.gorest-api/sdk/standard/client.gois excluded by!rest-api/sdk/standard/client.gorest-api/sdk/standard/model_measurement_trusted_machine.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_measurement_trusted_machine_create_request.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_measurement_trusted_profile.gois excluded by!rest-api/sdk/standard/model_*.gorest-api/sdk/standard/model_measurement_trusted_profile_create_request.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (7)
rest-api/api/pkg/api/handler/measurementtrust.gorest-api/api/pkg/api/handler/measurementtrust_test.gorest-api/api/pkg/api/model/measurementtrust.gorest-api/api/pkg/api/model/measurementtrust_test.gorest-api/api/pkg/api/routes.gorest-api/api/pkg/api/routes_test.gorest-api/openapi/spec.yaml
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rest-api/api/pkg/api/model/measurementtrust.go (1)
85-99: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse named
validation.Byvalidators for the cross-field ID rules.
MachineID != "*"and the matching delete-path selector logic should live in receiver methods, then be composed throughvalidation.Field(..., validation.By(...))instead of inline branching. That keeps the validation reusable and consistent with the rest of the model package.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rest-api/api/pkg/api/model/measurementtrust.go` around lines 85 - 99, Refactor APIMeasurementTrustedMachineCreateRequest.Validate to move the MachineID wildcard/UUID rule into a named receiver validator method and apply it through validation.Field with validation.By. Apply the same pattern to the matching delete-path selector logic, preserving the existing "*" allowance and UUID validation behavior while removing inline branching.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@rest-api/api/pkg/api/model/measurementtrust.go`:
- Around line 85-99: Refactor APIMeasurementTrustedMachineCreateRequest.Validate
to move the MachineID wildcard/UUID rule into a named receiver validator method
and apply it through validation.Field with validation.By. Apply the same pattern
to the matching delete-path selector logic, preserving the existing "*"
allowance and UUID validation behavior while removing inline branching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b876a0c1-d3a2-4ebe-8d70-719e0c2c86f2
📒 Files selected for processing (4)
rest-api/api/pkg/api/handler/measurementtrust.gorest-api/api/pkg/api/handler/measurementtrust_test.gorest-api/api/pkg/api/model/measurementtrust.gorest-api/api/pkg/api/model/measurementtrust_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- rest-api/api/pkg/api/handler/measurementtrust_test.go
- rest-api/api/pkg/api/handler/measurementtrust.go
|
@kfelternv same comments about PR description, far too verbose. |
What changedThis PR adds Provider Admin REST and generated Go SDK operations to create, list, and delete measured-boot machine and profile trust approvals. Verification covers all six operations, both delete selectors for each resource, both approval types, wildcard machine approval, the existing-profile requirement, authorization, and request validation at revision Scenario and setupThe NICo DevSpace stack is purged and redeployed from the exact PR revision. A Provider Admin performs the changed operations; a Tenant Admin checks the authorization boundary. The approval tables start empty. Profile approvals require a measured-boot system profile, but this PR does not add system-profile REST CRUD. A uniquely named prerequisite profile is therefore created with Run the command blocks in Bash. Bearer-token values are sanitized as environment variables; the remaining setup, REST, SDK, and cleanup commands are directly pasteable. Setup command: devspace purge -n nico-system
devspace deploy --skip-build -n nico-system
RUN_DIR="$HOME/Developer/_agent-tmp/pr3464-verification"
mkdir -p "$RUN_DIR"
kubectl -n nico-rest port-forward service/nico-rest-api 18388:8388 >"$RUN_DIR/api-port-forward.log" 2>&1 &
API_PORT_FORWARD_PID=$!
kubectl -n nico-rest port-forward service/keycloak 18082:8082 >"$RUN_DIR/keycloak-port-forward.log" 2>&1 &
KEYCLOAK_PORT_FORWARD_PID=$!
export REST_BASE=http://localhost:18388
export API="$REST_BASE/v2/org/test-org/nico/measured-boot"
export PROVIDER_TOKEN="<Provider Admin bearer token>"
export TENANT_TOKEN="<Tenant Admin bearer token>"
export PROFILE_NAME=pr3464-template-20260722-213740
export MISSING_PROFILE_ID="$(python3 -c 'import uuid; print(uuid.uuid4())')"
export SITE_ID="$(
curl -fsS --retry 10 --retry-connrefused --retry-delay 1 \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$REST_BASE/v2/org/test-org/nico/site" |
jq -er 'if type == "array" then .[0].id // .[0].siteId else .items[0].id // .items[0].siteId end'
)"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile?siteId=$SITE_ID"
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
'SELECT count(*) FROM measurement_approved_machines;'
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
'SELECT count(*) FROM measurement_approved_profiles;'
PROFILE_CREATE="$(
kubectl -n nico-system exec deployment/nico-api -- \
/opt/carbide/nico-admin-cli \
-a https://nico-api.nico-system.svc.cluster.local:1079 \
-f json \
attestation measured-boot profile create "$PROFILE_NAME" nvidia pr3464-template
)"
printf '%s\n' "$PROFILE_CREATE"
export PROFILE_ID="$(jq -r '.profile_id' <<<"$PROFILE_CREATE")"
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
"SELECT count(*) FROM measurement_system_profiles WHERE name = '$PROFILE_NAME';"Setup result: VerificationStep 1: Machine approval REST lifecycleWhy this step exists: It exercises machine POST, GET, and DELETE, including wildcard targets, both approval types, both delete selectors, response-field preservation, and not-found behavior. Command or action: curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
-H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"machineId\":\"*\",\"approvalType\":\"Persist\",\"pcrRegisters\":\"0,2,7\",\"comments\":\"pr3464-machine-persist\"}" \
"$API/trusted-machine"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine/%2A?siteId=$SITE_ID&selector=MachineId"
curl -sS -o "$RUN_DIR/m2-post.json" -w 'HTTP_STATUS=%{http_code}\n' \
-X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
-H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"machineId\":\"*\",\"approvalType\":\"Oneshot\",\"pcrRegisters\":\"1,4\",\"comments\":\"pr3464-machine-oneshot\"}" \
"$API/trusted-machine"
jq -c . "$RUN_DIR/m2-post.json"
export MACHINE_APPROVAL_ID_2="$(jq -r '.approvalId' "$RUN_DIR/m2-post.json")"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine/$MACHINE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine/$MACHINE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"Observed result: Why this proves the behavior: The new REST routes created both approval types, GET returned the exact persisted record, both Step 2: Profile approval REST lifecycleWhy this step exists: It exercises profile POST, GET, and DELETE, both approval types, both delete selectors, response-field preservation, and the Core existing-profile guard. Command or action: curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
-H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"$PROFILE_ID\",\"approvalType\":\"Oneshot\",\"pcrRegisters\":\"0,7\",\"comments\":\"pr3464-profile-oneshot\"}" \
"$API/trusted-profile"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile/$PROFILE_ID?siteId=$SITE_ID&selector=ProfileId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile/$PROFILE_ID?siteId=$SITE_ID&selector=ProfileId"
curl -sS -o "$RUN_DIR/p2-post.json" -w 'HTTP_STATUS=%{http_code}\n' \
-X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
-H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"$PROFILE_ID\",\"approvalType\":\"Persist\",\"pcrRegisters\":\"2,4\",\"comments\":\"pr3464-profile-persist\"}" \
"$API/trusted-profile"
jq -c . "$RUN_DIR/p2-post.json"
export PROFILE_APPROVAL_ID_2="$(jq -r '.approvalId' "$RUN_DIR/p2-post.json")"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile/$PROFILE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile/$PROFILE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
-H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"$MISSING_PROFILE_ID\",\"approvalType\":\"Persist\"}" \
"$API/trusted-profile"Observed result: Why this proves the behavior: The profile routes created both approval types for the setup profile, GET returned the exact persisted record, both Step 3: Authorization and validationWhy this step exists: Every new route is Provider Admin-only, and the new request models define approval-type, identifier, and resource-specific selector validation. Command or action: curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $TENANT_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"machineId\":\"*\",\"approvalType\":\"Persist\"}" \
"$API/trusted-machine"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $TENANT_TOKEN" \
"$API/trusted-machine?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X DELETE \
-H "Authorization: Bearer $TENANT_TOKEN" \
"$API/trusted-machine/$MACHINE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $TENANT_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"$PROFILE_ID\",\"approvalType\":\"Persist\"}" \
"$API/trusted-profile"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $TENANT_TOKEN" \
"$API/trusted-profile?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X DELETE \
-H "Authorization: Bearer $TENANT_TOKEN" \
"$API/trusted-profile/$PROFILE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ApprovalId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"machineId\":\"*\",\"approvalType\":\"Forever\"}" \
"$API/trusted-machine"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"$PROFILE_ID\",\"approvalType\":\"Forever\"}" \
"$API/trusted-profile"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"machineId\":\"not-a-uuid\",\"approvalType\":\"Persist\"}" \
"$API/trusted-machine"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X POST \
-H "Authorization: Bearer $PROVIDER_TOKEN" -H 'Content-Type: application/json' \
--data "{\"siteId\":\"$SITE_ID\",\"profileId\":\"not-a-uuid\",\"approvalType\":\"Persist\"}" \
"$API/trusted-profile"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine/$MACHINE_APPROVAL_ID_2?siteId=$SITE_ID&selector=ProfileId"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' -X DELETE \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile/$PROFILE_APPROVAL_ID_2?siteId=$SITE_ID&selector=MachineId"Observed result: Why this proves the behavior: The Tenant Admin received the same Provider Admin authorization error from every new operation. The Provider Admin's malformed requests reached each new validation branch and returned the expected resource-specific Step 4: Generated Go SDK lifecycleWhy this step exists: The generated SDK is a separately changed public interface and must be exercised as a real consumer, not inferred from raw REST success. Command or action: Observed result: Why this proves the behavior: A standalone consumer imported the generated SDK from the tested revision. Its machine and profile service methods created, listed, and deleted real approvals through the deployed REST API, preserved every supplied field, and left both lists empty. Cleanup action and result: kubectl -n nico-system exec deployment/nico-api -- /opt/carbide/nico-admin-cli -a https://nico-api.nico-system.svc.cluster.local:1079 -f json attestation measured-boot profile delete "$PROFILE_ID" --is-id
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-machine?siteId=$SITE_ID"
curl -sS -w '\nHTTP_STATUS=%{http_code}\n' \
-H "Authorization: Bearer $PROVIDER_TOKEN" \
"$API/trusted-profile?siteId=$SITE_ID"
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
"SELECT count(*) FROM measurement_system_profiles WHERE name = '$PROFILE_NAME';"
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
'SELECT count(*) FROM measurement_approved_machines;'
kubectl -n postgres exec postgres-0 -- psql -U postgres -d nico -Atc \
'SELECT count(*) FROM measurement_approved_profiles;'
kill "$API_PORT_FORWARD_PID" "$KEYCLOAK_PORT_FORWARD_PID"
PROFILE_DELETE={"profile_id":"$PROFILE_ID","name":"pr3464-template-20260722-213740","ts":"2026-07-23T02:52:30.636704Z","attrs":[{"key":"product_name","value":"pr3464-template"},{"key":"sys_vendor","value":"nvidia"}]}
SETUP_PROFILE_COUNT=0
FINAL_MACHINE_LIST=200 []
FINAL_PROFILE_LIST=200 []
FINAL_DB_MACHINE_COUNT=0
FINAL_DB_PROFILE_COUNT=0
DEPLOYED_REST_IMAGE=localhost:5000/nico-rest-api:72f8f677-1784753577
NICO_REST_RUNNING=8 NOT_READY=0 RESTARTS=0
NICO_SYSTEM_RUNNING=6 NOT_READY=0 RESTARTS=0
API_PORT_FORWARD=stopped
KEYCLOAK_PORT_FORWARD=stopped |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-core/src/measured_boot/tests/rpc.rs (1)
1644-1741: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid NotFound coverage; minor DRY opportunity on the repeated assertions.
The four removal scenarios each repeat
.await.unwrap_err(); assert_eq!(x.code(), tonic::Code::NotFound);. A tiny local helper would trim the boilerplate, though this is optional given each case calls a distinctly-typed handler/request.♻️ Optional helper to reduce repetition
+fn assert_not_found<T>(result: Result<T, tonic::Status>) { + assert_eq!(result.unwrap_err().code(), tonic::Code::NotFound); +} + #[crate::sqlx_test] async fn test_remove_measurement_trust_approvals(Then each of the four call sites becomes e.g.
assert_not_found(site::handle_remove_measurement_trusted_machine(api, ...).await);.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api-core/src/measured_boot/tests/rpc.rs` around lines 1644 - 1741, Optionally reduce repetition in test_remove_measurement_trust_approvals by adding a local helper that accepts the handler result and asserts tonic::Code::NotFound, then reuse it for the four missing-machine and missing-profile removal scenarios while preserving their distinct typed requests and handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/api-core/src/measured_boot/tests/rpc.rs`:
- Around line 1644-1741: Optionally reduce repetition in
test_remove_measurement_trust_approvals by adding a local helper that accepts
the handler result and asserts tonic::Code::NotFound, then reuse it for the four
missing-machine and missing-profile removal scenarios while preserving their
distinct typed requests and handlers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f9bece7a-7b1a-4a69-834b-7aaf46411582
📒 Files selected for processing (3)
crates/api-core/src/measured_boot/rpc/site.rscrates/api-core/src/measured_boot/tests/rpc.rscrates/api-db/src/measured_boot/interface/site.rs
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Adds Provider Admin REST operations for creating, listing, and deleting measured-boot trusted-machine and trusted-profile approvals. This exposes the existing Core capability through the established Temporal, SiteAgent, and Core gRPC path, including the OpenAPI contract and generated Go SDK.
Related issues
Part of #2801.
Type of Change
Breaking Changes
Testing
Additional Notes
The nicocli work in #2801 is not included in this PR.